-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changed markdown parser to as markdown didnt match gitlab. #721
base: develop
Are you sure you want to change the base?
Conversation
… needs to be CommonMark compliant
var buf bytes.Buffer | ||
renderer := goldmark.New( | ||
goldmark.WithRendererOptions(html.WithUnsafe()), | ||
) | ||
if err := renderer.Convert(md, &buf); err != nil { | ||
log.Error().Msgf("Error converting to commonmark: %s", err) | ||
} | ||
unsafeHtml := buf.Bytes() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's make this part as function and just call it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha im so stupid, i'll do that as soon i get time! :D
goldmark.WithRendererOptions(html.WithUnsafe()), | ||
) | ||
if err := renderer.Convert(md, &buf); err != nil { | ||
log.Error().Msgf("Error converting to commonmark: %s", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log.Error().Msgf("Error converting to commonmark: %s", err) | |
log.Error().Err(err).Msg("converting to markdown") |
Use the fields of the log package, might be nice if decided to do central logging for the distributed version later on as it makes it nicer to filter logs
goldmark.WithRendererOptions(html.WithUnsafe()), | ||
) | ||
if err := renderer.Convert(md, &buf); err != nil { | ||
log.Error().Msgf("Error converting to commonmark: %s", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my other comments
goldmark.WithRendererOptions(html.WithUnsafe()), | ||
) | ||
if err := renderer.Convert(md, &buf); err != nil { | ||
log.Error().Msgf("Error converting to commonmark: %s", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again log and errors shouldn't start with error
Reconsider if it makes sense to use |
I think when i created this i pretty much went from a zero trust perspective. And i think it is a good idea to keep it as a zero trust. For the goldmark with unsafe html, i will take a look, i havn't really done anything else thank making it work :) |
Changed markdown renderer, apparently old renderer was not CommonMark compliant. So the mark down parsed from readme on gitlab did not look the same in the webclient.